Don't infer closure signatures with late-bound type vars #108827
Conversation
|
r? @jackh726 (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
|
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
| @@ -1,8 +1,6 @@ | |||
| // Unit test for the "user substitutions" that are annotated on each | |||
| // node. | |||
|
|
|||
| // compile-flags:-Zverbose | |||
There was a problem hiding this comment.
This test's output changes with the modification to how bound/placeholder tys print under -Zverbose, though arguably I could revert that change -- just made debug a bit easier for me.
| @@ -760,7 +760,21 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { | |||
| }; | |||
|
|
|||
| let trait_ref = self.closure_trait_ref_unnormalized(obligation, substs); | |||
| let mut nested = self.confirm_poly_trait_refs(obligation, trait_ref)?; | |||
|
|
|||
| // FIXME(non_lifetime_binders): Perform the equivalent of a "leak check" here. | |||
There was a problem hiding this comment.
Alternatively, we could let this succeed, then error out during HIR typeck or something, though it would be harder to point to the source of the error.
|
@fmease: It ICEs with the same message, but for a different reason (the stacks have the same "head" but different "tails"). I can look into fixing that one too. |
|
Ah, I see. Should I open a separate issue for it (unless you'd like to fix it in this PR of course)? |
| }; | ||
|
|
||
| // FIXME(non_lifetime_binders): Higher-ranked Fn trait candidates are not (yet) supported. | ||
| // Make sure that the inputs/output don't capture any placeholder types. |
There was a problem hiding this comment.
I disagree with this. Canonicalization also maps ty::Param to a placeholder.
I generally think that this check should happen somewhere else 🤔
| if inferred_sig.visit_with(&mut MentionsTy { expected_ty }).is_continue() { | ||
| // FIXME(non_lifetime_binder): Don't infer a signature with late-bound ty/ct vars | ||
| if inferred_sig.visit_with(&mut MentionsTy { expected_ty }).is_continue() | ||
| && !inferred_sig.has_non_region_late_bound() |
There was a problem hiding this comment.
is this check not enough to prevent ICEs here? also, can you check that !inferred_sig.has_placeholders because inferring a closure signature to contain placeholders is wrong as the closure type is used outside of the binder which created them
There was a problem hiding this comment.
No, this just makes sure that we just don't eagerly deduce a closure signature from pending obligations.
We can still infer placeholders when we process pending obligations later, so we'd need to do something after typechecking to make sure that the closures don't capture any type placeholders.
There was a problem hiding this comment.
I agree that check here along with !inferred_sig.has_placeholders should be enough.
We keep getting ICE because of #109505 and probably because we're not using ty vars in the root universe.
|
r? @lcnr |
|
Hmm... yeah, then this needs to go back to the drawing table then @rustbot author |
rustbot
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #109442) made this pull request unmergeable. Please resolve the merge conflicts. |
Fixes #108814